feat(cli): honest non-interactive init — fail on skew, no false success (rc.2 M4)#687
Merged
Conversation
🦋 Changeset detectedLatest commit: 20e5df9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthrough
ChangesInit outcome handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant initCommand
participant installDepsStep
participant installEqlStep
participant installCommand
participant AgentHandoff
User->>initCommand: run stash init
initCommand->>installDepsStep: check dependency versions
installDepsStep-->>initCommand: dependency state or CliExit
initCommand->>installEqlStep: install EQL
installEqlStep->>installCommand: request installation
installCommand-->>installEqlStep: installed or migration-generated
installEqlStep-->>initCommand: EQL state
initCommand->>AgentHandoff: build prompt from installed skills
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/commands/impl/steps/handoff-claude.ts`:
- Around line 41-48: Make all generated handoff guidance respect the
installed-skills state: in handoff-claude.ts, update the setup prompt loaded by
the launchPrompt flow so the empty-skills variant does not reference skill
directories or the unwritten AGENTS.md file, while retaining valid skill
guidance when skills exist; in handoff-codex.ts, update its generated guidance
so .codex/skills/ is omitted when installed.length is zero.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8da61ce-6475-4788-b2d3-6295b53bb730
📒 Files selected for processing (9)
.changeset/honest-noninteractive-init.mdpackages/cli/src/commands/impl/steps/handoff-claude.tspackages/cli/src/commands/impl/steps/handoff-codex.tspackages/cli/src/commands/init/__tests__/init-command.test.tspackages/cli/src/commands/init/index.tspackages/cli/src/commands/init/steps/__tests__/install-deps.test.tspackages/cli/src/commands/init/steps/install-deps.tspackages/cli/src/messages.tsskills/stash-cli/SKILL.md
…ss (rc.2 M4) Non-interactive `stash init` reported success for setups that didn't fully complete. Three honesty gaps closed: 1. Version skew. A non-interactive run can't reconcile a `behind` skew (it won't mutate an install without consent), so instead of warning-and- proceeding — scaffolding against packages older than this CLI expects and then claiming success — it now REFUSES with a non-zero exit (CliExit(1)) and the exact align command. The #661/#666 no-mutation principle is preserved (execSync still not called); only the post-warning action changes from proceed to fail. `ahead` skew stays a warn (install likely fine; update the CLI). 2. False 'Setup complete'. The summary header and the auth/db/scaffold checkmarks were unconditional. Now: when EQL is required but not installed (eqlInstalled=false AND integration !== prisma-next), the summary reads 'Setup incomplete' and init exits non-zero, pointing at `stash eql install`. 'Database connection verified' → 'Database URL resolved' (init resolves a URL, never opens a connection). 'Encryption client scaffolded' is shown only when a client was written (clientFilePath set; skipped for prisma-next). 3. False 'skills loaded'. The Claude/Codex handoff launch prompt referenced the skills dir unconditionally; a stripped build installs no skills, so it told the agent to read files that aren't there. The clause is now conditional on installSkills() actually copying something. Tests: install-deps skew tests updated to assert the refusal (still never mutates); new init-command honest-summary tests (EQL-missing → exit 1 + 'Setup incomplete'; prisma-next eqlInstalled=false → completes). Skill + doc comments updated. Suite 542 unit / 62 e2e green, code:check clean. Changeset: stash minor. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
coderdan
force-pushed
the
feat/honest-noninteractive-init
branch
from
July 18, 2026 12:26
27520d6 to
fb77e14
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes stash init reliable for automation by ensuring non-interactive runs never report success when setup is incomplete or potentially inconsistent with the CLI’s expected runtime package versions.
Changes:
- Make non-interactive init refuse (exit non-zero) on behind version skew instead of warning-and-proceeding.
- Make the init summary truthful: “Database URL resolved” (not “connection verified”), “Setup incomplete” + exit non-zero when EQL is missing (except Prisma Next), and only reference skills when they were actually installed.
- Update unit coverage, changeset, and the published
stash-cliskill documentation to match the new behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| skills/stash-cli/SKILL.md | Documents the new “fail on behind skew” and “setup incomplete” non-interactive behavior. |
| packages/cli/src/messages.ts | Adds stable message leaders for non-interactive init failure modes. |
| packages/cli/src/commands/init/steps/install-deps.ts | Refuses non-interactively on behind skew (throws CliExit(1)), while keeping ahead skew as warn-and-proceed. |
| packages/cli/src/commands/init/steps/tests/install-deps.test.ts | Updates skew tests to assert refusal + no mutation in non-interactive mode. |
| packages/cli/src/commands/init/index.ts | Adjusts checkmark copy, adds EQL-missing “Setup incomplete” gate + non-zero exit (except Prisma Next). |
| packages/cli/src/commands/init/tests/init-command.test.ts | Adds unit tests asserting the honest-summary behavior and Prisma Next exception. |
| packages/cli/src/commands/impl/steps/handoff-codex.ts | Makes the “skills” clause in the launch prompt conditional on actual skill installation. |
| packages/cli/src/commands/impl/steps/handoff-claude.ts | Makes the “skills” clause in the launch prompt conditional on actual skill installation. |
| .changeset/honest-noninteractive-init.md | Adds release notes describing the non-interactive honesty changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two "only claim what actually happened" gaps found in review: - Copilot: the "✓ Encryption client scaffolded" checkmark was gated on `state.clientFilePath`, which build-schema sets even when it KEEPS an existing file (writes nothing). Gate on `state.schemaGenerated` (true only when a placeholder was written); show "✓ Encryption client kept (existing file)" for the keep path. Prisma Next (no clientFilePath) still shows neither. - CodeRabbit: the setup-prompt "Skills loaded" section pointed at skill directories and `AGENTS.md` regardless of what the handoff wrote — so a stripped build (no skills) sent the agent to read files that don't exist, and the Claude handoff (which never writes AGENTS.md) named it as the doctrine source. New `skillsLoadedLines(handoff, installedSkills)` renders the section honestly: no skills → point only at what was written (AGENTS.md for codex/agents-md, the docs for claude-code); claude-code → doctrine is in the installed skills, not AGENTS.md. Also hedged "list the skills loaded (if any)". Added setup-prompt honesty tests (empty-skills × handoff) and de-duped the two identical inline blocks. 557 tests green; build + biome clean. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Contributor
Author
Differential Review: PR #687Agent: Codex/gpt-5.6-sol Executive Summary
Overall risk: High Recommendation: Changes requested. The new completion gate still reports a Key metrics:
What ChangedCommit range: Timeline: 2026-07-17 to 2026-07-18
FindingsHigh: Drizzle still exits successfully before EQL is installedFile: Commit: Blast radius: Every Test coverage: Missing end-to-end state coverage. The new init test injects The new gate trusts
This preserves the exact false-success contract the PR is intended to remove. Concrete scenario:
Recommendation: Make Test Coverage AnalysisAll 557 CLI unit tests and all 65 CLI E2E tests passed. The CLI package build The important uncovered transition is:
Current tests cover each endpoint with mocks, which is why the contradictory Blast Radius Analysis
Historical Context
RecommendationsBlocking
Follow-up
Analysis MethodologyStrategy: Focused review of a large TypeScript repository (692 source files), Techniques used:
Limitations: No live database or credentials were used. The Drizzle finding Confidence: High. |
coderdan
commented
Jul 19, 2026
coderdan
left a comment
coderdan
left a comment
Contributor
Author
There was a problem hiding this comment.
Test-coverage review — stash init honesty changes (PR #687)
The behaviour that carries the exit-code contract is well covered: the non-interactive behind-skew refusal (exit 1, never mutates), the ahead-skew non-interactive non-refusal, the unreadable-manifest → behind path (install-deps.test.ts), and the EQL-missing → "Setup incomplete"/exit-1 gate plus the Prisma-Next exemption (init-command.test.ts). Nice work locking the leaders the e2e suite asserts on.
Three new branches introduced by this diff have no test exercising them:
- The encryption-client summary line now forks three ways (
scaffolded/kept (existing file)/ nothing) and none of the outcomes is asserted. - & 3. The
skillsClauseempty-vs-populated branch in each agent handoff prompt (handoff-claude.ts,handoff-codex.ts) — the launch-prompt analog of thesetup-promptchange that did get a dedicated test.impl.test.tsmockshowToProceedStep.runout, so no test drives either handoff prompt.
Details inline.
Additional coverage gaps not posted inline (minor)
install-deps.ts:213— the refusal test asserts thep.notealign commands and exit, but not thatp.log.error(messages.init.skewNonInteractive)is emitted (the why message). Low value; the exit-1 is the contract.setup-prompt.tsskillsLoadedLines— theagents-md+ no-skills combination isn't tested directly; it shares thewroteAgentsMdbranch withcodex(which is tested), so it's covered by construction rather than by assertion.
No security concerns noted in this diff.
Differential review (Codex) found the completion gate still reported a successful, complete setup for `stash init --drizzle` before EQL was in the database: the Drizzle path only *generates* a v2 migration (applied later with `drizzle-kit migrate`), yet `installEqlStep` returned `eqlInstalled: true` for every non-throwing outcome — the exact false-success this PR removes elsewhere. - `installCommand` now returns a structured `InstallOutcome` (`installed` | `already-installed` | `migration-generated` | `dry-run`). - `installEqlStep` maps `migration-generated` to a new `eqlMigrationPending` state (EQL not in the DB yet) instead of `eqlInstalled`. - The init summary reports it honestly — "○ EQL migration generated — apply it with `drizzle-kit migrate`" — and excuses it from the "Setup incomplete" hard-fail (exit 0), the same treatment Prisma Next already gets. - Changeset + stash-cli skill updated to match. Test-gap review: added the previously-uncovered branches — - init summary's three-way encryption-client checkmark (scaffolded / kept); - the Drizzle migration-pending gate (cross-layer: outcome → step → summary); - handoff-claude / handoff-codex launch-prompt skills clause (stripped build drops the skills dir; codex keeps AGENTS.md). Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Contributor
Author
|
Addressed the High finding (Drizzle exits successfully before EQL is installed) in The gate no longer trusts a boolean that meant two different things. The init summary now handles that outcome honestly and separately, exactly as recommended:
On the test gap: rather than injecting the final boolean, coverage now meets in the middle at Deferred (your follow-up bullet): persisting init outcomes in |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts (1)
85-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test exercises the real Drizzle chain end-to-end (
db/install.ts→install-eql.ts→init/index.ts). Each layer's test mocks its immediate dependency, so a future contributor could break theInstallOutcome→eqlMigrationPending→ completion-gate contract at any hop without a failing test — the same class of bug this PR fixes.
packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts#L85-L104: keep this unit test, but add a companion test that callsinstallEqlStep.runwith the realinstallCommand(mock only its internals — e.g.generateDrizzleMigration,pg/EQLInstaller) to prove the real function actually returns'migration-generated'for the Drizzle path and thatinstallEqlStepcorrectly maps it.packages/cli/src/commands/init/__tests__/init-command.test.ts#L118-L141: add a companion test that runsinitCommandwith the realinstallEqlStep.run(noteqlRun), mocking onlyinstallCommand, to prove the field set by the real step actually satisfiesinitCommand's completion gate for the Drizzle flow.Based on the PR's own review comments recommending "adding a cross-layer regression test covering the Drizzle flow through the final init status."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts` around lines 85 - 104, The Drizzle regression coverage stops at mocked layer boundaries and does not verify the real status propagation chain. In packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts:85-104, retain the existing unit test and add a companion using the real installCommand while mocking only its internals, asserting the real Drizzle path returns migration-generated and installEqlStep maps it to eqlMigrationPending. In packages/cli/src/commands/init/__tests__/init-command.test.ts:118-141, add a companion using the real installEqlStep.run while mocking only installCommand, and assert initCommand's final Drizzle completion status honors the field produced by the real step.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts`:
- Around line 85-104: The Drizzle regression coverage stops at mocked layer
boundaries and does not verify the real status propagation chain. In
packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts:85-104,
retain the existing unit test and add a companion using the real installCommand
while mocking only its internals, asserting the real Drizzle path returns
migration-generated and installEqlStep maps it to eqlMigrationPending. In
packages/cli/src/commands/init/__tests__/init-command.test.ts:118-141, add a
companion using the real installEqlStep.run while mocking only installCommand,
and assert initCommand's final Drizzle completion status honors the field
produced by the real step.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0544bc31-27b7-4347-acd8-e699865a706d
📒 Files selected for processing (17)
.changeset/honest-noninteractive-init.mdpackages/cli/src/commands/db/install.tspackages/cli/src/commands/impl/steps/__tests__/handoff-claude.test.tspackages/cli/src/commands/impl/steps/__tests__/handoff-codex.test.tspackages/cli/src/commands/impl/steps/handoff-claude.tspackages/cli/src/commands/impl/steps/handoff-codex.tspackages/cli/src/commands/init/__tests__/init-command.test.tspackages/cli/src/commands/init/index.tspackages/cli/src/commands/init/lib/__tests__/setup-prompt.test.tspackages/cli/src/commands/init/lib/setup-prompt.tspackages/cli/src/commands/init/steps/__tests__/install-deps.test.tspackages/cli/src/commands/init/steps/__tests__/install-eql.test.tspackages/cli/src/commands/init/steps/install-deps.tspackages/cli/src/commands/init/steps/install-eql.tspackages/cli/src/commands/init/types.tspackages/cli/src/messages.tsskills/stash-cli/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/cli/src/messages.ts
- packages/cli/src/commands/impl/steps/handoff-claude.ts
- packages/cli/src/commands/impl/steps/handoff-codex.ts
- .changeset/honest-noninteractive-init.md
- packages/cli/src/commands/init/index.ts
Contributor
Author
|
Human and AI review (copilot, coderabbit, Claude/Opus and Codex/gpt-5.6-sol) |
Contributor
Author
|
Tracked by #714. |
coderdan
added a commit
that referenced
this pull request
Jul 21, 2026
…s, and ship the doctrine where the CLI can find it (#745) * fix(cli): survive an unwritable .codex/, and inline skills when it happens Codex sandboxes deny writes under `.codex/`. `installSkills` created its destination with an unguarded `mkdirSync` sitting directly ABOVE a per-skill copy loop that was already guarded — so the failure threw past that fallback and past the caller, aborting the whole handoff step. Because the skills install runs first, nothing after it ran either: no AGENTS.md, no .cipherstash/context.json, no .cipherstash/setup-prompt.md. All five Codex runs of the rc.3 skilltester matrix landed here, and that report named it the primary driver of the Claude→Codex quality gap (#736). Two changes. `installSkills` never throws. Every filesystem step degrades to a warning and a shorter return, so the caller decides what an empty result means rather than being skipped entirely. The Codex handoff falls back to inlining. When the skills cannot be written, their bodies go into AGENTS.md — project root, writable — through the same `doctrine-plus-skills` path the editor-agent handoff already uses for Cursor / Windsurf / Cline. No new mechanism; the existing one just was not reachable from this branch. The launch prompt points at wherever the guidance actually ended up. The fallback is only claimed when there is something to inline. A stripped build ships no skills at all, so it stays doctrine-only and says nothing — reporting a fallback that did not happen would be the same false-success bug #714 and #687 removed elsewhere in init. That is why `availableSkills` is split out: an empty install result alone cannot distinguish "could not write" from "nothing to write". @cipherstash/wizard carries its own copy of `installSkills` with the identical unguarded `mkdirSync` above the identical guarded copy loop. It targets `.claude/skills`, so the Codex sandbox case does not apply, but an unwritable destination crashed it the same way — guarded to match. Tests: 2 for the mkdirSync guard (including that the caller can still distinguish unwritable from empty), 6 for the handoff across all three states — copied, unwritable-so-inlined, and nothing-to-ship. 645 unit + 68 pty E2E pass; typecheck and build clean; 0 biome errors. Checked per packages/cli/AGENTS.md: no E2E asserts on these strings (the only codex assertion is the target list, unchanged), and no skill documents the install location, so neither needed updating. * fix(cli): make the Codex skills fallback deliver what it claims Address the 15 findings from the PR #745 review. The structural change: installSkills returns { copied, failed } instead of a flat list, so callers distinguish unwritable / stripped / partial outcomes without a second probe. The Codex handoff inlines exactly the failed skills — a partial copy inlines the missing ones and the launch prompt points at both locations. The production-blocking find: the bundled AGENTS.md doctrine shipped at dist/commands/init/doctrine, but findBundledDir probes ancestors of the compiled chunk in dist/bin/ — so every published build wrote the minimal AGENTS.md stub, and the inline fallback would have inlined nothing while claiming otherwise. The doctrine now lands at dist/doctrine, like the skills bundle, and buildAgentsMdBody honours doctrine-plus-skills even when the doctrine fragment is missing. Honesty and hardening around the fallback: - writeArtifacts takes a SkillsDelivery (installed/inlined/failed); context.json gains inlinedSkills and setup-prompt.md stops mislabelling an unwritable destination as a "stripped build" — on the Claude and AGENTS.md handoffs too - availableSkills filters on SKILL.md, the predicate the inliner actually uses, so "inlining N skills" never overstates - the AGENTS.md upsert (which refuses malformed sentinel pairs) and the bundled-file reads degrade to warnings instead of aborting the step before .cipherstash/ is written; skills only count as inlined when the write landed - the fallback warning announces the observed recovery instead of asserting an unverified cause, and flags a stale .codex/skills/ it could not refresh - a confirmed-then-failed wizard install is recorded in the wizard changelog instead of vanishing with the terminal output Tests pin the degrade-to-warning contract in both packages (the wizard suite is new), cover the partial-copy split end to end, and replace the vacuously-true warning assertions. The doctrine's "Where to read more", the build-agents-md docblock, and root AGENTS.md now describe the real mode consumers. * test(cli): make the partial-copy fixture deterministic across platforms The read-only-directory trigger was platform-dependent: on macOS cpSync unlinks the destination file first (EACCES from the 0o555 parent), but on Linux it overwrites the writable SKILL.md in place and the copy succeeds — so the test failed on CI while passing locally. Block the skill with a FILE where its directory must go instead: cpSync then throws ERR_FS_CP_DIR_TO_NON_DIR, a type-mismatch error raised before any permission-dependent operation, so it fires on every platform and as root. Same technique the neighbouring mkdir test already uses. Drops the skipIf guard the chmod approach needed. * style(cli): format the partial-copy fixture line
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (rc.2 M4)
Non-interactive
stash initreported success for setups that hadn't fully completed — so CI/agents couldn't trust its exit code or its summary. Three honesty gaps:behindskew (an installed@cipherstash/*older than this CLI expects) — it won't mutate an install without consent — so it warned and kept going, scaffolding config/client against mismatched packages, then printed "Setup complete".eqlInstalled=false), the header still said complete — and "✓ Database connection verified" was printed although init only resolves a URL, never connects.installSkills()copied nothing (stripped build) — telling it to read files that aren't there.Fix
behindskew → refuse with a non-zero exit (CliExit(1)) and the exact align command, instead of proceeding. The npm dist-tags deliver broken/stale packages: adapters' latest=0.0.0, stack latest=0.19.0; stash init installs unpinned #661/fix(cli): pin init's installs to the release's own package versions (#661) #666 no-mutation principle is preserved (execSyncstill never called — only the post-warning action changes).aheadskew stays a warn (install likely fine; update the CLI). Interactive runs still offer to align.!eqlInstalled && integration !== 'prisma-next'), the summary reads "Setup incomplete" and init exits non-zero, pointing atstash eql install. "Database connection verified" → "Database URL resolved"; "Encryption client scaffolded" shows only when a client was actually written (skipped for Prisma Next).installSkills()returning something.Scope notes
prisma-next migration apply, soeqlInstalled=falseis expected there and is NOT treated as incomplete (tested).Verification
install-depsskew tests (refuses + exit 1, still never mutates); newinit-commandhonest-summary tests (EQL-missing → exit 1 + "Setup incomplete"; prisma-nexteqlInstalled=false→ completes).code:checkclean. Changeset:stashminor. Skill (stash-cli) + doc comments updated.initneeds live auth + a DB (which is why there's no init e2e); the honest-summary and skew-fail logic is unit-covered.https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Summary by CodeRabbit
stash initin non-interactive mode now fails (“Setup incomplete” / non-zero exit) when EQL setup is missing or when dependency version skew can’t be safely reconciled, including printing the exact alignment command..*/skills/when skills weren’t installed.stash initdocumentation to clarify exact version pinning, honest non-interactive mismatch behavior, and EQL/migration status expectations.